Convert enzyme tests to React Testing Library with help from Claude Code#1974
Convert enzyme tests to React Testing Library with help from Claude Code#1974
Conversation
labkey-alan
left a comment
There was a problem hiding this comment.
Overall the conversions look correct, however, in many cases I've found that Claude replaced await waitForLifeCycle() with await act(async () => {});, and I think we should probably be waiting for something specific in order to prevent flaky tests from being an issue. In at least one test suite I was able to drop the await act entirely with no problem.
| ); | ||
|
|
||
| expect(document.querySelector('.fa-spinner')).not.toBeNull(); | ||
| await act(async () => {}); |
There was a problem hiding this comment.
There are 4 lines that look like this, I believe all are meant to replace calls to waitForLifecycle, they should be replaced with something more specific.
There was a problem hiding this comment.
Done. Claude replaced / updated them.
| data: fromJS({ value: '2022-02-12 11:58:54.385', formattedValue: '2022-02-12' }), | ||
| }; | ||
|
|
||
| describe('ExpirationDateColumnRenderer', () => { |
There was a problem hiding this comment.
This test emits an error Warning: validateDOMNesting(...): <td> cannot appear as a child of <div>, but does not fail. I think we should consider cleaning up the error in order to reduce test noise. I think in order to clean up the error you'll need to create a wrapper in the test for all cases except not tablecell. Something that looks like:
const TestWrapper = props => (
<table>
<tbody>
<tr>
<ExpirationDateColumnRenderer {...props} />
</tr>
</tbody>
</table>
);
| <UserLink userDisplayValue="Test display" />, | ||
| { serverContext: { user: TEST_USER_APP_ADMIN } } | ||
| ); | ||
| await act(async () => {}); |
There was a problem hiding this comment.
The test contains six different lines that look like this, but if you delete them, and remove the act import, the test still passes.
| } | ||
| ); | ||
|
|
||
| await act(async () => {}); |
There was a problem hiding this comment.
There are several of these lines in this test suite. I think the test should probably be waiting for something specific or we're likely to have flaky tests.
labkey-alan
left a comment
There was a problem hiding this comment.
Most usages of await act(async () => {}); have been cleaned up, but there are trailing usages in SampleStatusInput.test.tsx that need to be addressed. Otherwise looks good.
| renderWithAppContext(<SampleStatusInput {...DEFAULT_PROPS} formsy={false} />, { | ||
| serverContext: { user: TEST_USER_STORAGE_EDITOR }, | ||
| }); | ||
| await act(async () => {}); // flush getSampleStatuses effect |
There was a problem hiding this comment.
There are 6 usages of await act(async () => {}); in this file.
Rationale
Major step towards finishing our conversion of our enzyme jest tests to RTL jest tests, using Claude Code for the conversion.
Related Pull Requests